Implement trip notifications and read state by buyolitsez · Pull Request #449 · BeWelcome/rox

buyolitsez · 2026-06-27T08:49:53Z

To close #340

notify nearby hosts when a member creates a new public trip
add a trip notification preference during signup/preferences
allow hosts to hide trips from their visitors lists
mark matching trip notifications as checked when a trip is hidden

telegram-cloud-photo-size-2-5330044562141026979-y

telegram-cloud-photo-size-2-5330044562141026982-y

telegram-cloud-photo-size-2-5330044562141027003-y

thisismeonmounteverest

@buyolitsez Looks good. There are a few problems, though.

The notification needs to be on the subtrip as the reason for 'reading' a trip might be either the person it self or the time of the visit.

The notification tab is a left over from far, far away and will be removed in the near future so we do not need to worry about that anymore.

The original discussion was to allow for digests: Immediately, daily, weekly, monthly. Starting with immediately is fine for me, but with that in mind the code might need some tinkling.

To find the members to be informed we should make use of the new spatial indices for geonames, shouldn't we?

Do we want to allow members to 'read' a leg on the visitors page (/visitors) or only on the landing page?

thisismeonmounteverest · 2026-06-29T19:30:23Z

+        $this->addSql('CREATE TABLE IF NOT EXISTS member_subtrip_read (id INT AUTO_INCREMENT NOT NULL, member_id INT NOT NULL, subtrip_id INT NOT NULL, created DATETIME NOT NULL, INDEX IDX_B1EC0277597D3FE (member_id), INDEX IDX_B1EC027F2BD7DD7 (subtrip_id), UNIQUE INDEX member_subtrip_read_unique (member_id, subtrip_id), CONSTRAINT FK_B1EC0277597D3FE FOREIGN KEY (member_id) REFERENCES member (id) ON DELETE CASCADE, CONSTRAINT FK_B1EC027F2BD7DD7 FOREIGN KEY (subtrip_id) REFERENCES sub_trips (id) ON DELETE CASCADE, PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
+        $this->addSql('CREATE TABLE IF NOT EXISTS member_trip_notification_sent (id INT AUTO_INCREMENT NOT NULL, member_id INT NOT NULL, trip_id INT NOT NULL, created DATETIME NOT NULL, INDEX IDX_9D9311917597D3FE (member_id), INDEX IDX_9D931191A5BC2E0E (trip_id), UNIQUE INDEX member_trip_notification_sent_unique (member_id, trip_id), CONSTRAINT FK_9D9311917597D3FE FOREIGN KEY (member_id) REFERENCES member (id) ON DELETE CASCADE, CONSTRAINT FK_9D931191A5BC2E0E FOREIGN KEY (trip_id) REFERENCES trips (id) ON DELETE CASCADE, PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');
        $this->addSql("INSERT INTO preferences (position, codeName, codeDescription, Description, created, DefaultValue, PossibleValues, Status) SELECT 65, 'TripsNotifications', 'trips.notifications', 'How often the member wants notifications for trips in their area', NOW(), 'No', 'No;Immediately;Daily;Weekly;Monthly', 'Normal' WHERE NOT EXISTS (SELECT 1 FROM preferences WHERE codeName = 'TripsNotifications')");
        $this->addSql("UPDATE preferences SET PossibleValues = 'No;Immediately;Daily;Weekly;Monthly' WHERE codeName = 'TripsNotifications'");


This would only change the database if the previous statement didn't. For new setups that will always be the case (the situation that needs cleanup can only arise during development).

thisismeonmounteverest · 2026-06-29T19:33:48Z

+        }
+
+        [$preferenceValue, $period] = self::FREQUENCIES[$frequency];
+        $until = new DateTimeImmutable();


I always thought about trip notifications as information about upcoming trips. This seems to target trips in the past.

thisismeonmounteverest · 2026-06-29T19:35:23Z

+use DateTime;
+use Doctrine\ORM\Mapping as ORM;
+
+#[ORM\Table(name: 'member_trip_notification_sent')]


As you hide legs of a trip notifications should be sent for legs as well, shouldn't they?

thisismeonmounteverest · 2026-06-29T19:37:37Z

@@ -151,25 +159,87 @@ public function markSubtripAsRead(Member $member, Subtrip $subtrip): void
    }

    public function notifyHostsAboutTrip(Trip $trip): void


The important information is that there is a traveller visiting your area. Which means we probably want to notifyHostsAboutVisitors(SubTrip $leg).

thisismeonmounteverest · 2026-06-29T19:44:19Z

+        return $sentCount;
+    }
+
+    private function acquireTripNotificationLock(Member $member, Trip $trip): ?string


Which part of the database is locked here? If more than the member_leg_notification_sent table this might impact the whole site.

thisismeonmounteverest · 2026-06-29T19:46:48Z

@@ -37,7 +37,6 @@ class Preference
    public const TRIP_NOTIFICATIONS = 'TripsNotifications';
    public const TRIP_NOTIFICATIONS_NEVER = 'No';


Should be Never.

thisismeonmounteverest · 2026-06-29T19:49:08Z

@@ -0,0 +1,29 @@
+<?php


I'd expect all things trips to be in the same repository.

thisismeonmounteverest · 2026-06-29T19:52:51Z

@@ -0,0 +1,32 @@
+<?php


This doesn't even use the database, right? So why is it a repository?

thisismeonmounteverest · 2026-06-29T19:53:24Z

        $entityManager = $this->createMock(EntityManagerInterface::class);
        $entityManager
-            ->expects($this->once())
+            ->expects($this->exactly(2))


This binds the test against implementation which isn't necessary.

thisismeonmounteverest · 2026-06-29T19:56:25Z

    }

-    private function createSubtripRepository(array $hosts): EntityRepository
+    private function createSubtripRepository(array $hosts): MatchingHostsRepository


A MatchingHostsRepository is not a SubtripRepository.

buyolitsez added 2 commits June 27, 2026 11:23

Implement trip notifications and read state

42bfe60

Fix notification test style

5e78ffd

thisismeonmounteverest requested changes Jun 27, 2026

View reviewed changes

buyolitsez added 2 commits June 28, 2026 12:41

Implement trip host notifications

11f79d1

Improve AboutModel mutation coverage

385b10c

thisismeonmounteverest reviewed Jun 28, 2026

View reviewed changes

Comment thread Migrations/Version20260626100000.php Outdated

thisismeonmounteverest reviewed Jun 28, 2026

View reviewed changes

Comment thread Migrations/Version20260626100000.php Outdated

thisismeonmounteverest requested changes Jun 28, 2026

View reviewed changes

buyolitsez added 3 commits June 28, 2026 17:40

Implement scheduled trip notifications

003901a

Rename subtrip read flow to hide

bf65c23

Update notification and conversation tests

357e4f6

thisismeonmounteverest requested changes Jun 29, 2026

View reviewed changes

		@@ -151,25 +159,87 @@ public function markSubtripAsRead(Member $member, Subtrip $subtrip): void
		}

		public function notifyHostsAboutTrip(Trip $trip): void

		@@ -37,7 +37,6 @@ class Preference
		public const TRIP_NOTIFICATIONS = 'TripsNotifications';
		public const TRIP_NOTIFICATIONS_NEVER = 'No';

Uh oh!

Conversation

buyolitsez commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thisismeonmounteverest left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

buyolitsez commented Jun 27, 2026 •

edited

Loading

thisismeonmounteverest left a comment •

edited

Loading